feat(AgmGeocoder): add geocoder service and tests #1743

DefJunx · 2019-10-11T07:23:04Z

add geocoder service
add geocoder unit tests
add geocoder related typing

implements #1694

codecov · 2019-10-11T07:30:39Z

Codecov Report

Merging #1743 into master will increase coverage by 2.76%.
The diff coverage is 92.68%.

@@            Coverage Diff             @@
##           master    #1743      +/-   ##
==========================================
+ Coverage   47.68%   50.45%   +2.76%     
==========================================
  Files          41       43       +2     
  Lines        1839     1881      +42     
  Branches      167      170       +3     
==========================================
+ Hits          877      949      +72     
+ Misses        958      928      -30     
  Partials        4        4

Impacted Files	Coverage Δ
packages/core/services.ts	`0% <0%> (ø)`	⬆️
packages/core/map-types.ts	`0% <0%> (ø)`
packages/core/services/geocoder-service.ts	`100% <100%> (ø)`
packages/core/services/google-maps-types.ts	`100% <100%> (+100%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c16e666...b9503e1. Read the comment docs.

ghost

Hey, thanks for the contribution. Please go over some things i've pointed out. Thanks.

packages/core/services/google-maps-types.ts

packages/core/services/geocoder-service.ts

packages/core/services/geocoder-service.spec.ts

DefJunx · 2019-10-17T12:26:02Z

Hi @doom777, thank you very much for your feedback. It really helps me improving my code.
I'll make sure to address the issues you've pointed out and fix them.

- Export new map types in the correct place Addresses PR sebholstein#1743 review

DefJunx · 2019-10-17T13:07:48Z

@doom777 i've done the required changes and pushed them to the branch.

DefJunx · 2019-10-17T13:28:02Z

@doom777 I also rebased together all the commits, leaving the implementation message.

Again, you might hate me after all these notifications but please don't!

dockleryxk · 2019-10-17T14:37:48Z

Just wanted to say this looks really good. Thanks for doing it!

- add geocoder service - add interfaces to support geocoder request and response - add test cases Implements sebholstein#1694

ghost

just one more change. it didnt work for me without it.

packages/core/services/geocoder-service.ts

- add geocoder service - add interfaces to support geocoder request and response - Add check to see if the geocoder instance is defined; if not, return an empty array of results

- add initial test implementation implements SebastianM#1694

- Fix wrong geocodeMock jest function implements SebastianM#1694

- avoid method being called without google being initialized - Add geocoder interface Implements sebholstein#1694

- Changed geocoder service structure Implements sebholstein#1694

- add geocoder object as private observable to follow style of other services Implements sebholstein#1694

- add test implements sebholstein#1694

ghost · 2019-10-17T17:12:33Z

hey, you missed my latest change request

…the office

- Add providedIn flag to service Injectable decorator

DefJunx · 2019-10-17T17:26:18Z

@doom777 no no, I saw it. though I had a fight with git because for some reason, when I pulled my changes on another machine I screwed everything up so I had to do some work to fix it.

Let me know if everything is in place now, it should be I think.

ghost

I came upon an efficiency problem. Solution is in the comments.

packages/core/services/geocoder-service.ts

packages/core/services/geocoder-service.spec.ts

and patch tests - Remove unwanted test expectation - Change geocoder Observable implementation to avoid instantiating multiple geocoder object

- Remove unwanted test expectation - Change geocoder Observable implementation to avoid instantiating multiple geocoder object

- Simplify creation of connectableGeocoder observable

- add geocoder service - add interfaces to support geocoder request and response - add test cases Implements: sebholstein#1694

DefJunx mentioned this pull request Oct 11, 2019

Geocoding Service #1694

Closed

ghost added the needs: review label Oct 13, 2019

ghost suggested changes Oct 17, 2019

View reviewed changes

DefJunx pushed a commit to DefJunx/angular-google-maps that referenced this pull request Oct 17, 2019

feat(AgmGeocoder): Export required Map types

de5ca6a

- Export new map types in the correct place Addresses PR sebholstein#1743 review

DefJunx requested a review from a user October 17, 2019 13:16

DefJunx force-pushed the geolocator-service branch from 997b842 to 2826406 Compare October 17, 2019 13:24

DefJunx force-pushed the geolocator-service branch from dbaad6a to fda0113 Compare October 17, 2019 14:26

feat(AgmGeocoder): add geocoder service

6ee3de7

- add geocoder service - add interfaces to support geocoder request and response - add test cases Implements sebholstein#1694

DefJunx force-pushed the geolocator-service branch from 7dcd361 to 6ee3de7 Compare October 17, 2019 14:54

ghost suggested changes Oct 17, 2019

View reviewed changes

packages/core/services/geocoder-service.ts Outdated Show resolved Hide resolved

Daniele Cencig added 7 commits October 17, 2019 18:55

feat(AgmGeocoder): add geocoder service

d71295e

- add geocoder service - add interfaces to support geocoder request and response - Add check to see if the geocoder instance is defined; if not, return an empty array of results

feat(AgmGeocoder): add geocoder service

c173b87

- add initial test implementation implements SebastianM#1694

feat(AgmGeocoder): add geocoder service

00501da

- Fix wrong geocodeMock jest function implements SebastianM#1694

feat(AgmGeocoder): add geocoder service

8391090

- avoid method being called without google being initialized - Add geocoder interface Implements sebholstein#1694

feat(AgmGeocoder): add geocoder service

002c7e0

- Changed geocoder service structure Implements sebholstein#1694

feat(AgmGeocoder): add geocoder service

f1b985c

- add geocoder object as private observable to follow style of other services Implements sebholstein#1694

feat(AgmGeocoder): add geocoder service

70e252c

- add test implements sebholstein#1694

DefJunx force-pushed the geolocator-service branch from 70e252c to 6ee3de7 Compare October 17, 2019 17:08

ghost removed the needs: review label Oct 17, 2019

hero2frag@gmail.com added 2 commits October 17, 2019 19:16

reverting all the damage i have done by pulling from home instead of …

50abf4a

…the office

feat(AgmGeocoder): add providedIn config

1b6917f

- Add providedIn flag to service Injectable decorator

ghost suggested changes Oct 18, 2019

View reviewed changes

packages/core/services/geocoder-service.ts Outdated Show resolved Hide resolved

packages/core/services/geocoder-service.spec.ts Outdated Show resolved Hide resolved

fix(AgmGeocoder): Efficiency patch for geocoder

0e9092f

and patch tests - Remove unwanted test expectation - Change geocoder Observable implementation to avoid instantiating multiple geocoder object

Daniele Cencig added 3 commits October 18, 2019 10:11

fix(AgmGeocoder): geocoder init fix and fix tests

fd9453c

- Remove unwanted test expectation - Change geocoder Observable implementation to avoid instantiating multiple geocoder object

fix(AgmGeocoder): Simplify connectable geocoder

b26c07c

- Simplify creation of connectableGeocoder observable

fix(AgmGeocoder): Remove unused defer

b9503e1

ghost approved these changes Oct 18, 2019

View reviewed changes

ghost merged commit 3549ccf into sebholstein:master Oct 18, 2019

DefJunx deleted the geolocator-service branch October 18, 2019 09:27

Waniusza mentioned this pull request Nov 18, 2019

When new release? #1764

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(AgmGeocoder): add geocoder service and tests #1743

feat(AgmGeocoder): add geocoder service and tests #1743

DefJunx commented Oct 11, 2019

codecov bot commented Oct 11, 2019 •

edited

Loading

ghost left a comment

DefJunx commented Oct 17, 2019 •

edited

Loading

DefJunx commented Oct 17, 2019

DefJunx commented Oct 17, 2019

dockleryxk commented Oct 17, 2019

ghost left a comment

ghost commented Oct 17, 2019

DefJunx commented Oct 17, 2019

ghost left a comment

feat(AgmGeocoder): add geocoder service and tests #1743

feat(AgmGeocoder): add geocoder service and tests #1743

Conversation

DefJunx commented Oct 11, 2019

codecov bot commented Oct 11, 2019 • edited Loading

Codecov Report

ghost left a comment

Choose a reason for hiding this comment

DefJunx commented Oct 17, 2019 • edited Loading

DefJunx commented Oct 17, 2019

DefJunx commented Oct 17, 2019

dockleryxk commented Oct 17, 2019

ghost left a comment

Choose a reason for hiding this comment

ghost commented Oct 17, 2019

DefJunx commented Oct 17, 2019

ghost left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 11, 2019 •

edited

Loading

DefJunx commented Oct 17, 2019 •

edited

Loading